Fix std430 layout support for cbuffers in .slang files #8432

Copilot · 2025-09-10T23:33:33Z

The layout(std430) cbuffer syntax was not working correctly in .slang files, causing incorrect ArrayStride values in generated SPIR-V. Arrays of int2 were getting ArrayStride 16 (std140 layout) instead of ArrayStride 8 (std430 layout).

The issue occurred because the parseHLSLCBufferDecl function only processed GLSL layout qualifiers when allowGLSLInput was enabled, but layout parsing itself is always available. This meant that while layout(std430) was parsed and attached as a modifier, it was ignored during cbuffer processing in .slang files.

Before:

OpDecorate %_arr_v2int_int_2 ArrayStride 16  // Wrong - using std140
OpName %SLANG_ParameterGroup_CBlock_std140 "SLANG_ParameterGroup_CBlock_std140"

After:

OpDecorate %_arr_v2int_int_2 ArrayStride 8   // Correct - using std430
OpName %SLANG_ParameterGroup_CBlock_std430 "SLANG_ParameterGroup_CBlock_std430"

This fix ensures that layout(std430) cbuffer declarations work consistently whether using .slang or .glsl files, and that the generated SPIR-V uses the correct layout rules for buffer members.

Additionally, the compiler now emits warnings when GLSL layout qualifiers (std140, std430, scalar) are used with non-Khronos targets (e.g., DXIL, HLSL, Metal), helping users identify potential compatibility issues without breaking existing code.

Fixes #8373.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: bmillsNV <[email protected]>

davli-nv · 2025-09-25T21:15:49Z

Copilot, I think it needs to warn when layout (std430) etc are used when compiling for other targets (warn instead of error to make it a non-breaking PR), and accept these layout modifiers when compiling for GLSL/SPIRV without requiring -allow-glsl

davli-nv · 2025-09-25T21:22:54Z

source/slang/slang-parser.cpp

-    // Check for GLSL layout qualifiers when GLSL input is allowed
-    if (parser->options.allowGLSLInput && parser->pendingModifiers)
+    // Check for GLSL layout qualifiers (layout modifiers can be used even without allowGLSLInput)
+    if (parser->pendingModifiers)


Copilot, I think it needs to warn when layout (std430) etc are used when compiling for other targets (warn instead of error to make it a non-breaking PR), and accept these layout modifiers when compiling for GLSL/SPIRV without requiring -allow-glsl

davli-nv · 2025-09-25T21:26:27Z

tests/spirv/cbuffer-std430-layout.slang

+    int2 a[2];
+};
+
+ConstantBuffer<B, Std430DataLayout> c;


Copilot, for simplicity this ConstantBuffer can be dropped

davli-nv · 2025-09-30T19:28:28Z

@claude can you update this PR based on review comments, or make new PR if needed

github-actions · 2025-09-30T19:43:30Z

Claude finished @davli-nv's task —— View job

I've successfully updated the PR based on the review comments! ✅

Todo List:

Analyze current implementation and review comments
Understand Slang's target system and warning mechanisms
Implement warning system for layout qualifiers on incompatible targets
Modify parser to accept layout modifiers for GLSL/SPIRV without -allow-glsl
Simplify test file by removing ConstantBuffer as requested
Build and test the changes
Commit and push updates

Implementation Summary:

✅ Warning system: Added diagnostic ID 39012 that warns when std430, std140, or scalar layout qualifiers are used on incompatible targets (non-Khronos/WGPU). The warning is issued during semantic analysis where target information is available.

✅ Layout modifier acceptance: The existing code already accepts layout modifiers without requiring -allow-glsl flag (as noted in the comment: "layout modifiers can be used even without allowGLSLInput"), so this requirement was already met.

✅ Test simplification: Removed the ConstantBuffer<B, Std430DataLayout> c; line and related usage as requested in the review.

Changes Made:

slang-diagnostic-defs.h: Added diagnostic 39012 for layout qualifier warnings
slang-check-modifier.cpp: Added warning logic that checks compilation targets and issues warnings for incompatible ones
cbuffer-std430-layout.slang: Simplified test by removing ConstantBuffer usage

The implementation ensures that:

Layout qualifiers work correctly on GLSL/SPIRV targets (non-breaking)
Warnings (not errors) are issued for incompatible targets (non-breaking)
Existing functionality is preserved while addressing the review feedback

The changes have been committed and pushed to the PR branch.

…atible targets - Add diagnostic 39012 for layout qualifiers on unsupported targets - Implement warning system that checks compilation targets and warns when std430/std140/scalar layout qualifiers are used on non-Khronos/WGPU targets - Simplify test by removing ConstantBuffer usage as requested in review - Layout qualifiers are already accepted without -allow-glsl flag (existing behavior) Addresses review comments on PR #8432. Co-authored-by: davli-nv <[email protected]>

davli-nv · 2025-10-03T00:10:40Z

source/slang/slang-check-modifier.cpp

+    {
+        String layoutName;
+
+        if (as<GLSLStd140Modifier>(layoutMod))


this can use existing modifier->keywordName

Done in commit 51d89ee. Now using layoutMod->keywordName instead of manual string construction.

davli-nv · 2025-10-03T00:11:10Z

source/slang/slang-check-modifier.cpp

+            if (!isKhronosTarget(target->getTarget()) && !isWGPUTarget(target->getTarget()))
+            {
+                String targetName;
+                switch (target->getTarget())


is there existing function to get human string name of targets?

Done in commit 51d89ee. Now using the existing printDiagnosticArg function to get human-readable target names.

davli-nv · 2025-10-03T01:12:05Z

tests/spirv/cbuffer-std430-layout.slang

@@ -0,0 +1,15 @@
+//TEST:SIMPLE(filecheck=SPIRV): -target spirv -entry main -stage fragment -emit-spirv-directly


add a second case in same test file that checks for warning with other target

Done in commit 51d89ee. Added a second test case that targets DXIL and checks for the warning: warning 39012: layout qualifier 'std430' is not supported for target 'dxil'

davli-nv · 2025-10-03T01:13:13Z

@copilot update this PR based on review comments

…warning test Co-authored-by: davli-nv <[email protected]>

davli-nv · 2025-10-03T02:40:51Z

@copilot can you show how slang handles layout std430 etc for targeting wgpu?

Copilot · 2025-10-03T02:59:29Z

@copilot can you show how slang handles layout std430 etc for targeting wgpu?

For WGPU targets (WGSL, WGSLSPIRV, WGSLSPIRVAssembly), Slang handles layout qualifiers as follows:

No warnings are issued when using layout(std430) with WGPU targets - the code at line 1686 explicitly exempts WGPU targets: !isWGPUTarget(target->getTarget())
For storage buffers and texture buffers: WGPU uses std430 layout rules by default (WGSLLayoutRulesFamilyImpl::getShaderStorageBufferRules() and getTextureBufferRules() return kStd430LayoutRulesImpl_)
For constant buffers (cbuffers): WGPU uses a custom kWGSLConstantBufferLayoutRulesImpl_ which is based on std140 layout rules (line 2291-2292 in slang-type-layout.cpp)
Layout capabilities: The Std430DataLayout type is marked with [require(spirv)] and [require(glsl)] in hlsl.meta.slang, so explicit std430 layout is only available when targeting SPIRV/GLSL, not pure WGSL. However, WGPU targets that go through SPIRV (like WGSLSPIRV) can use the layout qualifiers.

Initial plan

4c07e6a

Copilot AI assigned Copilot and bmillsNV Sep 10, 2025

Copilot started work on behalf of bmillsNV September 10, 2025 23:33 View session

Copilot AI and others added 3 commits September 10, 2025 23:52

Initial analysis: Identified the issue with std430 cbuffer ArrayStride

2d3b783

Co-authored-by: bmillsNV <[email protected]>

Fix std430 layout support for cbuffers in .slang files

441e3cd

Co-authored-by: bmillsNV <[email protected]>

Fix std430 layout support for cbuffers in .slang files

3651590

Co-authored-by: bmillsNV <[email protected]>

Copilot AI changed the title ~~[WIP] [SPIRV] wrong ArrayStride inside layout(std430) cbuffer~~ Fix std430 layout support for cbuffers in .slang files Sep 11, 2025

Copilot AI requested a review from bmillsNV September 11, 2025 00:06

Copilot finished work on behalf of bmillsNV September 11, 2025 00:06

csyonghe mentioned this pull request Sep 25, 2025

[SPIRV] wrong ArrayStride inside layout(std430) cbuffer #8373

Open

davli-nv requested changes Sep 25, 2025

View reviewed changes

davli-nv self-assigned this Sep 25, 2025

davli-nv requested changes Oct 3, 2025

View reviewed changes

Copilot started work on behalf of davli-nv October 3, 2025 01:13 View session

Address review feedback: Use keywordName and printDiagnosticArg, add …

51d89ee

…warning test Co-authored-by: davli-nv <[email protected]>

Copilot AI requested a review from davli-nv October 3, 2025 01:47

Copilot finished work on behalf of davli-nv October 3, 2025 01:47

Copilot started work on behalf of davli-nv October 3, 2025 02:40 View session

Copilot finished work on behalf of davli-nv October 3, 2025 03:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix std430 layout support for cbuffers in .slang files #8432

Fix std430 layout support for cbuffers in .slang files #8432

Copilot AI commented Sep 10, 2025 •

edited

Loading

Uh oh!

davli-nv commented Sep 25, 2025

Uh oh!

davli-nv Sep 25, 2025

Uh oh!

davli-nv Sep 25, 2025

Uh oh!

davli-nv commented Sep 30, 2025

Uh oh!

github-actions bot commented Sep 30, 2025 •

edited

Loading

Uh oh!

davli-nv Oct 3, 2025

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

davli-nv Oct 3, 2025

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

davli-nv Oct 3, 2025 •

edited

Loading

Uh oh!

Copilot AI Oct 3, 2025

Uh oh!

davli-nv commented Oct 3, 2025

Uh oh!

davli-nv commented Oct 3, 2025

Uh oh!

Copilot AI commented Oct 3, 2025

Uh oh!

Uh oh!

		@@ -0,0 +1,15 @@
		//TEST:SIMPLE(filecheck=SPIRV): -target spirv -entry main -stage fragment -emit-spirv-directly

Fix std430 layout support for cbuffers in .slang files #8432

Are you sure you want to change the base?

Fix std430 layout support for cbuffers in .slang files #8432

Conversation

Copilot AI commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davli-nv commented Sep 25, 2025

Uh oh!

davli-nv Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

davli-nv Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

davli-nv commented Sep 30, 2025

Uh oh!

github-actions bot commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo List:

Implementation Summary:

Changes Made:

Uh oh!

davli-nv Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

davli-nv Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

davli-nv Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

davli-nv commented Oct 3, 2025

Uh oh!

davli-nv commented Oct 3, 2025

Uh oh!

Copilot AI commented Oct 3, 2025

Uh oh!

Uh oh!

Copilot AI commented Sep 10, 2025 •

edited

Loading

github-actions bot commented Sep 30, 2025 •

edited

Loading

davli-nv Oct 3, 2025 •

edited

Loading